[COMPRESS-722] Introduce handling of entries compressed by a specific method#765
[COMPRESS-722] Introduce handling of entries compressed by a specific method#765uvoigt wants to merge 2 commits intoapache:masterfrom
Conversation
… method in ZipArchiveOutputStream - implicitly close entries when writing
|
@garydgregory is it possible to add copilot to the reviewers list to have a raw check? https://docs.github.com/en/copilot/how-tos/use-copilot-agents/request-a-code-review/use-code-review |
|
|
||
| if (entry != null) { | ||
| throw new ArchiveException("This archive contains unclosed entries."); | ||
| closeArchiveEntry(); |
There was a problem hiding this comment.
To keep the API compatible with older clients, wouldn’t it be better to execute this only when the isAutoCompressed flag is set and otherwise still throw the exception?
There was a problem hiding this comment.
We can close this.. There was a todo in the ArchiveOutputStreamTest which is resolved by this change, so it was somehow planned to do..
There was a problem hiding this comment.
It is tested in testAutoCompressZstd(ZipMethod zipMethod) for auto-compress and in testAutoCompressFalsePreservesLegacyBehavior() for user specified compressing.
| aos2.putArchiveEntry(aos2.createArchiveEntry(dummy, "dummy")); | ||
| aos2.write(dummy); | ||
|
|
||
| // TODO check if second putArchiveEntry() can follow without closeAE? |
There was a problem hiding this comment.
Ah.. sorry didn't saw that there is a todo.. Might be also a good time to fix this..
There was a problem hiding this comment.
This one can also be closed.
|
From my perspective, this looks good. All workarounds and edge cases seem to be covered and the implementation as well as the Javadocs are clear and understandable. |
uvoigt
force-pushed
the
feat/COMPRESS-722
branch
from
3ea743b to
6c8265a
Compare
|
Fixed a problem with the builder construction. I was not that familiar with |
…uilder construction without file and fix an inconsistency
uvoigt
force-pushed
the
feat/COMPRESS-722
branch
from
6c8265a to
6ae0742
Compare
|
Hi @garydgregory , do you need anything for this PR, or is something still missing? Best regards |
|
Hello @mehmet-karaman Please be patient. This is not the only issue or Commons component that needs attention 😉 |
Happy Easter! Hope you're enjoying some free time! Thank you for your work on these kinds of software projects! |
3 participants
Content
This introduces the mode
autoCompressofZipArchiveOutputStream. If enabled, the stream uses factories of the implemented methods to create a compressingOutputStreamper entry. When using this mode, it is no longer necessary to explicitly set the uncompressed size of an entry.Additional modifications:
Thanks for your contribution to Apache Commons! Your help is appreciated!
Before you push a pull request, review this list:
mvn; that'smvnon the command line by itself.